Pull request overview

This PR restores backward compatibility for the _use_legacy_endpoint=True feature flag across the Azure AI Evaluation RAI-service-backed evaluators and red team scoring, primarily by normalizing metric names between the legacy annotation endpoint and the newer sync_evals endpoint, and by aligning legacy conversation behavior with pre-sync-migration semantics.

Changes:

• Add bidirectional metric/request/response-key normalization in rai_service.py and update evaluator routing to skip double-parsing for legacy responses.
• Restore legacy whole-conversation evaluation behavior (single-call) and extend red team scorer/processor to pass endpoint choice and evaluator name for telemetry.
• Add/extend unit + e2e tests for legacy endpoint parity (query/response, conversation, and metric mapping).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.

    # Normalize metric name (same mapping as evaluate_with_rai_service_sync)
    metric_name_str = metric_name.value if hasattr(metric_name, "value") else metric_name
    if use_legacy_endpoint:
        legacy_name = _SYNC_TO_LEGACY_METRIC_NAMES.get(metric_name_str)
        if legacy_name:
            metric_name = legacy_name
    else:
        _legacy_to_sync = {v: k for k, v in _SYNC_TO_LEGACY_METRIC_NAMES.items()}
        sync_name = _legacy_to_sync.get(metric_name_str)
        if sync_name:
            metric_name = sync_name

    # Route to legacy endpoint if requested
    if use_legacy_endpoint:
        return await evaluate_with_rai_service_multimodal(
            messages=messages,
            metric_name=metric_name,
            project_scope=project_scope,
            credential=credential,
        )

You can also share your feedback on Copilot code review. Take the survey.

Code Review

🔴 Legacy conversation path returns wrong format (Critical)

_evaluate_conversation() legacy path (lines 140-147 of _base_rai_svc_eval.py) returns the raw result from evaluate_with_rai_service_sync_multimodal() directly — a flat dict like {"violence": 0, "violence_score": 0, ...}. But conversation evaluation is expected to return an evaluation_per_turn structure built by _aggregate_results(). The legacy path bypasses _aggregate_results() entirely.

The e2e test test_content_safety_evaluator_conversation_with_legacy_endpoint asserts:

assert "evaluation_per_turn" in score
assert len(score["evaluation_per_turn"]["violence"]) == 2

This will fail since the flat dict from the legacy endpoint has no evaluation_per_turn key. Either the legacy path needs to wrap its result through _aggregate_results(), or the test expectations need to match the actual legacy output format.

🟡 Missing metric_display_name in multimodal path (High)

evaluate_with_rai_service_sync() correctly sets metric_display_name = metric_display_name or metric_name_str before remapping to the legacy name, so output keys remain hate_unfairness_*. However, evaluate_with_rai_service_sync_multimodal() does NOT set metric_display_name:

# evaluate_with_rai_service_sync (correct):
if legacy_name:
    metric_display_name = metric_display_name or metric_name_str  # ✅ preserves original name
    metric_name = legacy_name

# evaluate_with_rai_service_sync_multimodal (missing):
if legacy_name:
    metric_name = legacy_name  # ❌ no metric_display_name preservation

This means legacy multimodal results will produce keys under the legacy name (hate_fairness_*) instead of the expected name (hate_unfairness_*).

Fix: Add metric_display_name = metric_display_name or metric_name_str in the multimodal function before metric_name = legacy_name.

🟡 Duplicated normalization logic (Medium)

The 10-line metric normalization block is copy-pasted in both evaluate_with_rai_service_sync() and evaluate_with_rai_service_sync_multimodal(). This duplication already caused bug #2 above (missing metric_display_name in one copy).

Fix: Extract to a helper:

def _normalize_metric_for_endpoint(metric_name, use_legacy_endpoint, metric_display_name=None):
    metric_name_str = metric_name.value if hasattr(metric_name, "value") else metric_name
    if use_legacy_endpoint:
        legacy_name = _SYNC_TO_LEGACY_METRIC_NAMES.get(metric_name_str)
        if legacy_name:
            return legacy_name, (metric_display_name or metric_name_str)
    else:
        sync_name = _LEGACY_TO_SYNC_METRIC_NAMES.get(metric_name_str)
        if sync_name:
            return sync_name, metric_display_name
    return metric_name, metric_display_name

🟡 Response key fallback applies unconditionally (Medium)

The legacy response key fallback in parse_response() and _parse_content_harm_response() applies even when NOT using the legacy endpoint:

if metric_name not in batch_response[0]:
    legacy_key = _SYNC_TO_LEGACY_RESPONSE_KEYS.get(...)
    if legacy_key and legacy_key in batch_response[0]:
        response_key = legacy_key  # runs even for sync endpoint

If the sync endpoint returns an unexpected format, this silently falls back to checking legacy keys instead of failing fast. This could mask API contract violations.

Fix: Thread use_legacy_endpoint through to these functions and only apply the fallback when use_legacy_endpoint=True.

Thanks for addressing the prior review. The helper extraction and _aggregate_results wrapping look good. Two remaining issues:

🔴 Conversation legacy test asserts wrong per-turn count (will fail)

test_content_safety_evaluator_conversation_with_legacy_endpoint asserts:

assert len(score["evaluation_per_turn"]["violence"]) == 2
assert len(score["evaluation_per_turn"]["violence_score"]) == 2

But the legacy path sends the entire conversation in a single call, then wraps the single result via _aggregate_results([result]). That produces evaluation_per_turn lists of length 1 (one aggregated result), not 2. The sync path evaluates per-turn (2 turns → length 2), but the legacy path intentionally evaluates as a single batch.

Fix: Either change assertions to == 1 (matching actual legacy behavior), or evaluate per-turn in legacy mode too (but that would contradict the stated goal of matching pre-migration behavior).

🟡 metric_display_name still not propagated in multimodal legacy path (High)

evaluate_with_rai_service_sync_multimodal discards the display name:

metric_name, _ = _normalize_metric_for_endpoint(metric_name, use_legacy_endpoint)

Downstream, evaluate_with_rai_service_multimodal() calls parse_response(annotation_response, metric_name) without metric_display_name, so output keys use the legacy name (hate_fairness_score, hate_fairness_reason) instead of the expected sync name (hate_unfairness_score, hate_unfairness_reason).

This affects HateUnfairnessEvaluator and GroundednessProEvaluator in conversation mode with legacy endpoint. It's untested because the only conversation+legacy test uses ViolenceEvaluator (no remapping needed).

Fix: Thread metric_display_name through to evaluate_with_rai_service_multimodal() and pass it to parse_response(). This requires adding the parameter to the multimodal function signature.

All four issues from previous reviews are resolved:

1. Duplicated normalization → Extracted to _normalize_metric_for_endpoint() helper ✅
2. metric_display_name in multimodal → Now threaded through full chain: _normalize_metric_for_endpoint → evaluate_with_rai_service_sync_multimodal → evaluate_with_rai_service_multimodal (new param) → parse_response ✅
3. Conversation legacy format → Wrapped via _aggregate_results([result]); test correctly asserts == 1 per-turn entries with explanatory comment ✅
4. Response key fallback scope → Comments clarify parse_response is inherently legacy-only ✅

LGTM.